Skip to content

Conversation

@sirtimid
Copy link
Contributor

@sirtimid sirtimid commented Apr 16, 2025

This PR modularizes our kernel code for improved organization and maintainability by:

  • Creating a dedicated KernelQueue to manage the kernel’s run queue.

  • Refactored message routing logic from Kernel into dedicated KernelRouter class with specialized delivery handlers for each message type, improving separation of concerns and type safety.

  • Extracting the syscall handling logic into a new VatSyscall class for processing vat-related syscalls such as sending messages, resolving promises, and dropping imports.

  • Moving translation methods from both Kernel and VatHandle into the store object, centralizing the translation of references and messages between kernel and vat spaces.

@sirtimid sirtimid requested a review from a team as a code owner April 16, 2025 13:51
@sirtimid sirtimid requested a review from grypez April 16, 2025 19:12
@sirtimid sirtimid changed the title refactor: Isolate Kernel Queue, Syscall & Translation Components refactor: Isolate Kernel Queue, Syscall, Router & Translation Components Apr 17, 2025
Copy link
Contributor

@grypez grypez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is an improvement to the organization. Judging by the number of yet unimplemented syscalls, I foresee VatSyscall.ts will need to be broken up further. Perhaps a good criteria for isolating the subcomponents will be whether the syscall requires access to the KernelQueue, KernelStore, or both.

Consider labelling the PR commit with refactor(kernel): since the changes are isolated to the kernel package.

Approved!

@grypez grypez requested a review from FUDCo April 17, 2025 16:37
Copy link
Contributor

@FUDCo FUDCo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pro forma to hold merge until I have a chance to look at this. Which I'll do promptly I promise.

Copy link
Contributor

@FUDCo FUDCo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor nit which you can decide if you want to handle it now or later or not at all. Other than that, looks good.

Comment on lines +129 to +141
/**
* Add an item to the tail of the kernel's run queue.
*
* @param item - The item to add.
*/
enqueueRun(item: RunQueueItem): void {
this.#kernelStore.enqueueRun(item);
if (this.#kernelStore.runQueueLength() === 1 && this.#wakeUpTheRunQueue) {
const wakeUpTheRunQueue = this.#wakeUpTheRunQueue;
this.#wakeUpTheRunQueue = null;
wakeUpTheRunQueue();
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are three enqueueXXX methods, but enqueueMessage and enqueueNotify are specific while enqueueRun is more generic (used both in the implementation of the other two and for general queue handling). So I'd suggest reordering them to put enqueueRun before of after the other two rather than in between them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok yeah I'll do it later.

@sirtimid sirtimid changed the title refactor: Isolate Kernel Queue, Syscall, Router & Translation Components refactor(kernel): Isolate Kernel Queue, Syscall, Router & Translation Components Apr 17, 2025
@sirtimid sirtimid merged commit 68fd6cf into main Apr 17, 2025
21 checks passed
@sirtimid sirtimid deleted the sirtimid/kernel-restructure branch April 17, 2025 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants